feat(someip): add server/provider side (offer services, answer RPC, publish events) - #913
feat(someip): add server/provider side (offer services, answer RPC, publish events)#913kirkbrauer wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe SOME/IP driver now supports provider/server mode. It can offer services, return configured RPC responses, publish events, set fields, list offers, manage server state, and use provider configuration and tests. ChangesSOME/IP provider support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SomeIpDriverClient
participant SomeIp
participant OsipServer
Client->>SomeIpDriverClient: configure provider behavior
SomeIpDriverClient->>SomeIp: call server API
SomeIp->>OsipServer: start, offer service, or register handler
OsipServer-->>SomeIp: deliver RPC request or subscriber event
SomeIp-->>Client: return response or publish event
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@vtz — as the original author of this SOME/IP driver, your review would be much appreciated. This adds the server/provider side (offer services via SD, answer RPC with canned responses, publish events) on top of your client-side implementation, wrapping the |
…ublish events) The SOME/IP driver was client-only. opensomeip already ships a full server-side API (SomeIpServer / SdServer / RpcServer / EventPublisher), so expose it through the driver so a Jumpstarter exporter can act as a simulated ECU that a device-under-test's SOME/IP client talks to. New exported verbs (driver + client): - start_server / stop_server: server lifecycle (lazily started on first offer) - offer_service / stop_offer_service / list_offered_services: SD offering - set_method_response / clear_method_response: canned RPC responses. RPC handlers run in the exporter process and cannot call back to the Jumpstarter client per request, so responses are configured declaratively per (service_id, method_id); this maps cleanly onto getter-style methods. - register_event / publish_event / set_field: event group notifications and cached field events. Adds a SomeIpOfferedService model, README "Server / Provider" usage section, a provider exporter example, and 13 server-side unit tests (83 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
756f728 to
f07ee6b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py (1)
359-369: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
try/except ValueErrorover the enum internal_value2member_map_.
ReturnCode._value2member_map_is a CPython enum implementation detail. Constructing the enum in atry/exceptis the documented idiom and is robust to future enum internals.♻️ Optional refactor
- payload, return_code = self._method_responses.get(key, (b"", int(ReturnCode.E_OK))) - rc = ReturnCode(return_code) if return_code in ReturnCode._value2member_map_ else ReturnCode.E_NOT_OK + payload, return_code = self._method_responses.get(key, (b"", int(ReturnCode.E_OK))) + try: + rc = ReturnCode(return_code) + except ValueError: + rc = ReturnCode.E_NOT_OK🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py` around lines 359 - 369, Update the ReturnCode conversion in handler to construct ReturnCode(return_code) inside a try block and catch ValueError to fall back to ReturnCode.E_NOT_OK. Remove the direct use of ReturnCode._value2member_map_, while preserving the existing response/error message type behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py`:
- Around line 359-369: Update the ReturnCode conversion in handler to construct
ReturnCode(return_code) inside a try block and catch ValueError to fall back to
ReturnCode.E_NOT_OK. Remove the direct use of ReturnCode._value2member_map_,
while preserving the existing response/error message type behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d0c1305-d16a-4d5a-83aa-e3bbee1e30f0
📒 Files selected for processing (6)
python/packages/jumpstarter-driver-someip/README.mdpython/packages/jumpstarter-driver-someip/examples/exporter.yamlpython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/client.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/common.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver_test.py
|
I will remove my review to let @vtz come and review. |
vtz
left a comment
There was a problem hiding this comment.
Thanks for this. Nice addition. Exposing the provider side so an exporter can act as a simulated ECU fits Open SOME/IP and opensomeip well, and one driver ≈ one ECU is the right Jumpstarter shape.
Inline comments cover the small cleanup/test nits. Separately, worth noting for later (not blockers): Fire&Forget and the full Field getter/setter/notifier model from the spec aren’t in scope here (canned RPC + set_field/events cover the usual simulator path).
Great work!
|
|
||
| @export | ||
| @validate_call(validate_return=True) | ||
| def publish_event(self, service_id: int, event_id: int, payload: SomeIpPayload) -> None: |
There was a problem hiding this comment.
service_id is accepted here but never forwarded (opensomeip only takes event_id). Please document that it’s unused for API symmetry, or remove it so callers aren’t misled.
- Revert formatting-only changes to existing client methods - Reject reserved instance IDs (0x0000, 0xFFFF) in offer_service - Clear canned method responses in stop_server so stale payloads do not survive a server restart - Document that service_id is unused (API symmetry) in publish_event and set_field on both driver and client - Add stateful loopback server tests (StatefulOsipServer + LoopbackOsipClient) covering offer + discovery + canned RPC responses, event publishing, and state clearing across stop_server Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restore main's original formatting in driver.py and driver_test.py (hex literal casing, line wrapping) so the diff only contains the server/provider feature changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver_test.py (1)
1233-1239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert
message_typein the error test.The driver returns
MessageType.ERRORwhen the return code is notE_OK(driver.py Lines 365-375). The test assertsreturn_codeonly. Add themessage_typeassertion to cover the error branch fully.♻️ Proposed test tightening
resp = loopback_client.rpc_call(0x1801, 0x0006, b"") assert resp.return_code == 0x01 + assert resp.message_type == int(MessageType.ERROR)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver_test.py` around lines 1233 - 1239, Update test_loopback_method_error_return_code to also assert that resp.message_type is MessageType.ERROR when the non-success return_code is returned, while preserving the existing return_code assertion.python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.py (1)
426-428: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake
stop()drop handler and event registrations.
stop()clears_offeredonly. It keepshandlers,registered_events, andfields. A real server restart drops all registrations. The loopback tests patchOsipServerwith a fixedreturn_value, so the same instance is reused afterstop_server(). Stale handlers therefore survive a restart that would clear them in production.This weakens
test_loopback_stop_server_clears_canned_responses: the post-restartrpc_callsucceeds only because the stale handler remains registered. A driver defect that skips method re-registration after a restart would not be detected.♻️ Proposed fix to align the double with real restart semantics
def stop(self): self._started = False self._offered.clear() + self.handlers.clear() + self.registered_events.clear() + self.fields.clear()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.py` around lines 426 - 428, Update the loopback server double’s stop() method to clear handlers, registered_events, and fields in addition to _offered, matching production restart semantics. Ensure all registration state is removed before the same OsipServer instance can be reused after stop_server().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.py`:
- Around line 485-490: Update the OsipMessage construction in the loopback
request helper to set explicit SOME/IP response headers: provide an
interface_version and message_type matching the values expected by
SomeIp._make_method_handler. Preserve the existing message_id, request_id, and
payload behavior.
---
Nitpick comments:
In
`@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.py`:
- Around line 426-428: Update the loopback server double’s stop() method to
clear handlers, registered_events, and fields in addition to _offered, matching
production restart semantics. Ensure all registration state is removed before
the same OsipServer instance can be reused after stop_server().
In
`@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver_test.py`:
- Around line 1233-1239: Update test_loopback_method_error_return_code to also
assert that resp.message_type is MessageType.ERROR when the non-success
return_code is returned, while preserving the existing return_code assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a8fdedc9-d154-424e-8422-0b3a0073b41f
📒 Files selected for processing (4)
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/client.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py
| request = OsipMessage( | ||
| message_id=OsipMessageId(sid, mid), | ||
| request_id=OsipRequestId(0x0001, len(self._rpc_history)), | ||
| payload=payload, | ||
| ) | ||
| return handler(request) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
opensomeip 0.1.5 python Message class fields interface_version message_type defaults
💡 Result:
In the opensomeip 0.1.5 library, the Message class is defined as a dataclass within the opensomeip.message module [1][2]. While specific default values for the interface_version and message_type fields are not explicitly documented in the public summary, they are standard components of the SOME/IP header [3]. It is important to distinguish the library opensomeip (v0.1.5) from other Python packages such as pysomeip [4][5]. The opensomeip library is primarily a Python wrapper/interface for a C++17 implementation [6][7]. For the SOME/IP protocol generally, these fields follow the specification [3]: - interface_version: An 8-bit field representing the Major Version of the Service Interface [3]. - message_type: An 8-bit field used to differentiate message types (e.g., REQUEST, RESPONSE, ERROR) [3]. If you are programmatically inspecting the opensomeip 0.1.5 Message dataclass, you can verify the default values in your local environment using the following Python snippet: import dataclasses from opensomeip.message import Message print(dataclasses.field_defaults(Message)) If you find that the opensomeip package does not expose these fields directly in the Python dataclass constructor due to its underlying C++ architecture, you may need to interface with the RpcClient or MessageId objects provided by the library to set these headers [1][2].
Citations:
- 1: https://pypi.org/project/opensomeip/0.1.5/
- 2: https://pypi.org/project/opensomeip/
- 3: https://some-ip.com/open-someip-specification/open_someip_specification_2025-12.pdf
- 4: https://pysomeip.readthedocs.io/en/latest/someip.html
- 5: https://github.com/afflux/pysomeip/blob/master/tools/get.py
- 6: https://github.com/vtz/opensomeip
- 7: https://vtz.github.io/opensomeip/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files =="
git ls-files python/packages/jumpstarter-driver-someip | sed -n '1,120p'
echo "== conftest relevant =="
fd -a 'conftest.py' python/packages/jumpstarter-driver-someip | while read -r f; do
rel="${f#$(pwd)/}"
echo "--- $rel"
wc -l "$rel"
sed -n '440,505p' "$rel"
done
echo "== driver relevant =="
for f in python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/*.py; do
case "$f" in
*driver.py | *client.py | *server.py) ;;
*) continue ;;
esac
echo "--- $f"
wc -l "$f"
rg -n "interface_version|request_id|message_type|return_code|OsipMessage\\(" "$f" -C 3 || true
doneRepository: jumpstarter-dev/jumpstarter
Length of output: 7080
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== opensomeip package version in requirements =="
rg -n "opensomeip|jumpstarter-driver-someip" python/pyproject.toml python/packages/jumpstarter-driver-someip/pyproject.toml poetry.lock uv.lock requirements*.txt -S 2>/dev/null || true
echo "== imports / constants in conftest driver and common =="
for f in python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.py \
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py \
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/common.py; do
echo "--- $f"
sed -n '1,80p' "$f"
done
echo "== inspect installed opensomeip if available =="
uv run python3 - <<'PY' 2>/dev/null || true
import inspect
try:
import opensomeip
print("opensomeip", opensomeip.__version__ if hasattr(opensomeip, "__version__") else getattr(opensomeip, "__version__", "?"))
except Exception as e:
print("import error:", e)
raise SystemExit
from opensomeip.message import Message, MessageId, OsipRequestId, ReturnCode, MessageType
print("Message", Message)
try:
print("Message sig", inspect.signature(Message))
except Exception as e:
print("Message sig error", e)
try:
from dataclasses import fields
for f in fields(Message):
print("field", f.name, "default", f.default, "default_factory", f.default_factory)
except Exception as e:
print("fields error", e)
try:
m = Message(message_id=MessageId(0x1234, 0x5678), request_id=OsipRequestId(1, 2), payload=b"X")
print("constructed", m)
print("fields", {k:getattr(m,k,None) for k in ["message_id","request_id","message_type","return_code","interface_version","payload","protocol_version"]})
except Exception as e:
print("constructed error", type(e).__name__ + ":", e)
PYRepository: jumpstarter-dev/jumpstarter
Length of output: 7823
🌐 Web query:
vtz opensomeip 0.1.5 Message source interface_version message_type
💡 Result:
The opensomeip library (v0.1.5) provides Python bindings for the open-source C++17 implementation of the Scalable service-Oriented MiddlewarE over IP (SOME/IP) protocol [1][2]. In the context of the SOME/IP protocol, which this library implements, the fields interface_version and message_type are standard components of the SOME/IP header [3][4]: 1. Interface Version: This is an 8-bit field representing the Major Version of the service interface [3][4]. It is used to manage service definitions, detect mismatches, and allow debugging tools to identify the specific interface version in use [4]. 2. Message Type: This is an 8-bit field used to identify the type of message being transmitted [3][4]. Common values include: - 0x00: REQUEST (expects a response) [4] - 0x01: REQUEST_NO_RETURN (fire-and-forget) [4] - 0x02: NOTIFICATION (event notification) [4] - 0x80: RESPONSE (a response message) [4] - 0x81: ERROR (an error response) [4] Additionally, the third-highest bit of the Message Type (0x20) acts as a TP-Flag, which, when set, indicates that the message is a segment for the SOME/IP-TP (Transport Protocol) [3]. For specific implementation details within the opensomeip library, you should refer to the official documentation and source code hosted at the project's repository [1][2].
Citations:
- 1: https://pypi.org/project/opensomeip/0.1.5/
- 2: https://github.com/vtz/opensomeip
- 3: https://some-ip.com/open-someip-specification/open_someip_specification_2025-12.pdf
- 4: https://some-ip.com/papers/drafts/2012-07-12-SOMEIP_Specification_Draft__ReRelease.pdf
🌐 Web query:
vtz opensomeip 0.1.5 Message __init__ Python dataclass source
💡 Result:
In the opensomeip Python package (v0.1.5), the Message class is implemented as a Python dataclass within the opensomeip.message module [1][2]. As a dataclass, its structure and initialization rely on standard Python dataclass mechanics [1]. While the exact source code for the init method is automatically generated by the @dataclass decorator at runtime, the class is defined to hold SOME/IP header fields and the message payload [1]. This design is part of the project's goal to provide a "Pythonic" API that utilizes plain Python objects rather than opaque C++ wrappers for its public-facing types [1][2]. To interact with this class in your code, you can import it as follows: from opensomeip.message import Message Since it is a dataclass, you initialize it by passing the required fields directly to the constructor (e.g., Message(message_id=..., request_id=...,...)) [1]. You can verify the specific fields available by inspecting the Message class definition in the opensomeip/message.py file within the source repository [3].
Citations:
- 1: https://pypi.org/project/opensomeip/0.1.5/
- 2: https://pypi.org/project/opensomeip/
- 3: https://github.com/vtz/opensomeip
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== opensomeip source from pypi tarball, if wget is available =="
tmp="$(mktemp -d)"
cd "$tmp"
if command -v wget >/dev/null 2>&1; then
wget -q https://files.pythonhosted.org/packages/source/o/opensomeip/opensomeip-0.1.5.tar.gz
tar -xzf opensomeip-0.1.5.tar.gz
if [ -f opensomeip-0.1.5/opensomeip/message.py ]; then
sed -n '1,260p' opensomeip-0.1.5/opensomeip/message.py
else
echo "message.py not found"
find opensomeip-0.1.5 -name '*.py' -maxdepth 3 | sort
fi
else
echo "wget unavailable"
fiRepository: jumpstarter-dev/jumpstarter
Length of output: 246
Set SOME/IP response headers for the loopback request.
OsipMessage is created with only message_id, request_id and payload, but SomeIp._make_method_handler reads request.interface_version and passes request.message_type when building the response. Set an explicit interface_version and message_type so this path does not rely on undefined defaults.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.py`
around lines 485 - 490, Update the OsipMessage construction in the loopback
request helper to set explicit SOME/IP response headers: provide an
interface_version and message_type matching the values expected by
SomeIp._make_method_handler. Preserve the existing message_id, request_id, and
payload behavior.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py \
--items all --type function,method --match '_make_method_handler|set_method_response'
rg -n -A35 -B8 'def _make_method_handler\b|def set_method_response\b' \
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py
rg -n -C3 '\bopensomeip\b' \
python/pyproject.toml \
python/packages/jumpstarter-driver-someip/pyproject.toml \
uv.lock 2>/dev/null || trueLength of output: 6177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
make lint-fix
make pkg-test-jumpstarter-driver-someip
make pkg-ty-jumpstarter-driver-someipLength of output: 470
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '475,495p' \
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.py
git diff -- \
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/conftest.pyLength of output: 1225
@kirkbrauer, the change is not present in the current branch. LoopbackOsipClient.call still omits both header arguments.
Apply this update:
request = OsipMessage(
message_id=OsipMessageId(sid, mid),
request_id=OsipRequestId(0x0001, len(self._rpc_history)),
interface_version=INTERFACE_VERSION,
message_type=MSG_TYPE_REQUEST,
payload=payload,
)Validation is inconclusive because make is unavailable in this environment. Run the required package checks in a development environment that has the project toolchain.
🧬 Code Graph Analysis Results
driver.py
- Lines 479–496:
register_eventstores event-to-event-group mappings and delegates registration to the SOME/IP server;publish_eventconverts hexadecimal payload data to bytes and publishes by event ID. - Lines 500–507:
set_fieldconverts hexadecimal payload data to bytes and delegates field updates to the SOME/IP server.service_idis accepted but unused.
driver_test.py
- Lines 930–971: Test fake server lifecycle and event APIs.
start/stoptoggle state;offer/stop_offermanage offered services;offered_servicesexposes service metadata;register_method,register_event,publish_event, andset_fieldrecord or store server operations.
client.py
- Lines 144–164: Client wrappers for event registration, event publication, and field updates. Payloads are hex-encoded into
SomeIpPayloadbefore invoking the corresponding RPC.
You are interacting with an AI system.
|
sent to merge queue again, the python failure was unrelated, related to renode. |
|
@mangelajo Humm, looks like it failed again, I'll wait for @vtz 's review first and then we can retry the merge queue :) |
The SOME/IP driver is currently client-only.
opensomeipalready ships a full server-side API (SomeIpServer/SdServer/RpcServer/EventPublisher), so this exposes it through the driver so a Jumpstarter exporter can act as a simulated ECU that a device-under-test's SOME/IP client talks to.New exported verbs (driver + client)
start_server/stop_server— server lifecycle (lazily started on first offer)offer_service/stop_offer_service/list_offered_services— SD offeringset_method_response/clear_method_response— canned RPC responsesregister_event/publish_event/set_field— event-group notifications and cached field eventsDesign note
RPC handlers run inside the exporter process (opensomeip receive thread) and cannot call back to the Jumpstarter client per request, so responses are configured declaratively per
(service_id, method_id)rather than via a per-request callback. This maps naturally onto getter-style SOME/IP methods, updating the response changes what a client reads without re-registration.Also included
SomeIpOfferedServicemodel for server-side introspectionopensomeip.SomeIpServer); full package suite: 83 passed, 2 skippedThe client side is unchanged; all additions are additive.